Skip to content

Conversation

@thomassedlmayer
Copy link
Contributor

@thomassedlmayer thomassedlmayer force-pushed the adopt-role-harmonization branch 3 times, most recently from d20c45a to fbb8506 Compare November 27, 2025 15:38
Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>
Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>
@thomassedlmayer thomassedlmayer force-pushed the adopt-role-harmonization branch from fbb8506 to c90791b Compare December 2, 2025 12:11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too familiar with the OSI structure, but in 3.7 I found the roles under "Vehicle Classification", see screenshot below. Is this something we need to change as we address all kinds of moving objects here?

Image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should also think about how to include it generically for all moving objects. But I though, we might want to harmonize the existing enum first.

We could just reference the existing enum definition in MovingObjectClassification or - for a cleaner result - move the role enum definition out of the VehicleClassification to be a generic enum definition (osi3::Role instead of osi3::MovingObject::VehicleClassification::Role) and then reference it in two places. I believe this should work in a backwards compatible way because we wouldn't change the data representation by moving enum definitions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't judge the implications on backwards compatibility of your suggestion for the cleaner result, but it does seem like the better / less confusing approach.

@thomassedlmayer thomassedlmayer marked this pull request as ready for review January 22, 2026 13:19
@thomassedlmayer thomassedlmayer added the ReadyForCCBReview Indicates that this PR is ready for a final review and merge by the CCB. label Jan 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ReadyForCCBReview Indicates that this PR is ready for a final review and merge by the CCB.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants